fix: error in modal background#815
Open
dennisvankekem wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a UI issue where the global <Error /> view can appear behind modals, by tracking open modals in global Redux state and conditionally rendering the error view only when no modal is open. It also extends InformationBanner to support an explicit error variant and adds unit tests around the new behavior.
Changes:
- Add
openModalCountto the global Redux slice withmodalOpened/modalClosedactions and unit tests. - Hide the global
<Error />component insrc/layouts/Paper.tsxwhile a modal is open. - Update
ConfigureGitModalto dispatch modal open/close tracking and refine banner styling (including an error banner), with a comprehensive modal test suite.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/redux/reducers.ts | Adds modal-open tracking to global Redux state and exports new actions. |
| src/redux/reducers.test.ts | Adds reducer unit tests for modal-open tracking. |
| src/layouts/Paper.tsx | Suppresses the global <Error /> rendering while a modal is open. |
| src/components/modals/ConfigureGitModal.tsx | Dispatches modal open/close actions; improves banner spacing and error banner type. |
| src/components/modals/ConfigureGitModal.test.tsx | Adds unit tests covering modal behavior including the new dispatch logic. |
| src/components/InformationBanner.tsx | Introduces type prop to render “info” vs “error” banner styling. |
| src/components/InformationBanner.test.tsx | Adds unit tests validating InformationBanner default and error variants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
4
to
+15
| export interface GlobalState { | ||
| error: any | ||
| isDirty: boolean | undefined | null | ||
| openModalCount: number | ||
| } | ||
|
|
||
| const initialState = { error: undefined, isDirty: undefined, isStale: false } as GlobalState | ||
| const initialState = { | ||
| error: undefined, | ||
| isDirty: undefined, | ||
| isStale: false, | ||
| openModalCount: 0, | ||
| } as GlobalState |
Comment on lines
+7
to
+10
| const StyledInfoBanner = styled(Box)<{ | ||
| small?: boolean | ||
| type: InformationBannerType | ||
| }>(({ small, type }) => ({ |
Comment on lines
+38
to
40
| <StyledInfoBanner small={small} type={type} sx={sx}> | ||
| <Iconify icon={icon} width={width} height={28} color={iconColor} /> | ||
| <Typography sx={{ color: theme.palette.text.primary, ml: 1 }}>{message}</Typography> |
|
|
||
| # Suppress global page error while any modal is open | ||
|
|
||
| To prevent visual conflict between modal content and page-level errors, the application tracks open modal state globally and suppresses rendering of the top-level Error component while at least one modal is open. We chose this over a ConfigureGit-only fix because the problem is cross-cutting, and we chose explicit modal lifecycle dispatch over an ad-hoc button handlers because it is more resilient to close/unmount edge cases. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Considerations